fix(sandbox): strip query string from L7 path matching#614
Closed
johntmyers wants to merge 1 commit intomainfrom
Closed
fix(sandbox): strip query string from L7 path matching#614johntmyers wants to merge 1 commit intomainfrom
johntmyers wants to merge 1 commit intomainfrom
Conversation
|
johntmyers
force-pushed
the
fix/l7-strip-query-from-path-matching
branch
from
88b8c97 to
fb96e25
Compare
drew
previously approved these changes
Mar 30, 2026
pimlock
previously approved these changes
Mar 30, 2026
Comment on lines
+123
to
+124
| // bytes (forwarded to upstream) are left untouched — only the policy | ||
| // evaluation fields are split. |
Collaborator
There was a problem hiding this comment.
Just out of curiosity I checked the untouched — only and it actually the em-dash (U+2014) vs regular ASCII hyphen. Based on definition, em-dash is more correct, but not something that is easy to type on a keyboard.
With agents writing these, that's actually okay. Commenting for awareness, I know using an em-dash is one indicator of AI generated content, but we are upfront about this :)
johntmyers
force-pushed
the
fix/l7-strip-query-from-path-matching
branch
from
21565f2 to
6541591
Compare
Previously, L7 policy path rules received the full request URI including the query string. This caused exact path rules like `path: /api/v1/download` to silently fail when clients added query parameters (e.g., `/api/v1/download?slug=foo`), because Rego's glob.match treats the query string as part of the last path segment. This fix splits the request target into path and query components during HTTP parsing. Path rules now match only the path component, and query strings are passed through transparently to the upstream server. This matches user expectations: a path rule controls which endpoints are reachable, not which query parameters are allowed. Changes: - Add `query` field to L7Request and L7RequestInfo structs - Split path/query in parse_http_request before L7 policy evaluation - Pass query string to Rego input for future query param filtering - Add l7_query field to L7_REQUEST log output - Add tests for query string splitting and path matching with query params - Document path matching behavior in policy-schema.md Refs: /discussions/607
johntmyers
force-pushed
the
fix/l7-strip-query-from-path-matching
branch
from
6541591 to
e6cda0d
Compare
Collaborator
Author
|
Closing as superseded by #617, which implemented the full query parameter matching feature including the path/query splitting fix this PR introduced. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix L7 policy path matching to ignore query strings, so exact path rules like
path: /api/v1/downloadcorrectly match requests with query parameters (e.g.,/api/v1/download?slug=foo&version=1.0).Related Issue
Refs: #607
Changes
queryfield toL7RequestandL7RequestInfostructs to hold the query string separately from the pathparse_http_requestl7_queryfield to L7_REQUEST log output for observabilityTesting
Smoke tested locally with httpbin.org:
GET /getGET /get?foo=bar&baz=quxGET /headersGET /anythingGET /anything?test=1mise run pre-commitpasses (excluding pre-existing license issues)Checklist